-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for indicator constraints #120
Add support for indicator constraints #120
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mohammadfawaz for the great PR! I added only one small comment, otherwise ready to merge.
rhs: f64, | ||
name: &str, | ||
) -> Rc<Constraint> { | ||
assert_eq!(vars.len(), coefs.len()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to have an assert here that the bin_var
is indeed binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the assert.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #120 +/- ##
==========================================
+ Coverage 74.96% 76.05% +1.09%
==========================================
Files 13 13
Lines 1725 1737 +12
==========================================
+ Hits 1293 1321 +28
+ Misses 432 416 -16 ☔ View full report in Codecov by Sentry. |
Closes #403 Most expressions are now supported. Calls, state stuff, and if expressions are not supported yet. The file `scip.rs` grow too much so I had to split it into multiple modules. I'm now using a forked version of `russcip` until [this PR](scipopt/russcip#120) makes it in and a release is published. That change adds "indicator constraints" which are critical for Boolean operators, especially `||`. Note that not every solver supports indicator constraints. For those solvers, we'd have to use a different approach: likely the ["big-M" method ](https://en.wikipedia.org/wiki/Big_M_method) which I've used in the past in my grad school research and found that it adds instability and performance issues. For now, SCIP is fine and likely the ideal option for the vast majority of problems.
Closes #119
Indicator constraints can now be added using:
which uses SCIP's
SCIPcreateConsBasicIndicator
under the hood.This is quite useful for manipulating all kinds of Boolean expressions such as OR constraints etc.